AMDGPU: Remove most manual AVLdSt decoder code #157861
Merged
+0
−42
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was additional hacking around using incorrect register class constraints for paired data operands. I'm not really sure why we need any of what's left. In particular the IS_VGPR special case seems backwards from how the encoding works.
Member
|
@llvm/pr-subscribers-backend-amdgpu Author: Matt Arsenault (arsenm) ChangesThis was additional hacking around using incorrect register class Full diff: https://github.com/llvm/llvm-project/pull/157861.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 6f6039bf4ec21..d3db1b7394675 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -368,51 +368,9 @@ static DecodeStatus decodeOperandVOPDDstY(MCInst &Inst, unsigned Val,
return addOperand(Inst, DAsm->decodeVOPDDstYOp(Inst, Val));
}
-static bool IsAGPROperand(const MCInst &Inst, int OpIdx,
- const MCRegisterInfo *MRI) {
- if (OpIdx < 0)
- return false;
-
- const MCOperand &Op = Inst.getOperand(OpIdx);
- if (!Op.isReg())
- return false;
-
- MCRegister Sub = MRI->getSubReg(Op.getReg(), AMDGPU::sub0);
- auto Reg = Sub ? Sub : Op.getReg();
- return Reg >= AMDGPU::AGPR0 && Reg <= AMDGPU::AGPR255;
-}
-
static DecodeStatus decodeAVLdSt(MCInst &Inst, unsigned Imm, unsigned Opw,
const MCDisassembler *Decoder) {
const auto *DAsm = static_cast<const AMDGPUDisassembler *>(Decoder);
- if (!DAsm->isGFX90A()) {
- Imm &= 511;
- } else {
- // If atomic has both vdata and vdst their register classes are tied.
- // The bit is decoded along with the vdst, first operand. We need to
- // change register class to AGPR if vdst was AGPR.
- // If a DS instruction has both data0 and data1 their register classes
- // are also tied.
- unsigned Opc = Inst.getOpcode();
- uint64_t TSFlags = DAsm->getMCII()->get(Opc).TSFlags;
- AMDGPU::OpName DataName = (TSFlags & SIInstrFlags::DS)
- ? AMDGPU::OpName::data0
- : AMDGPU::OpName::vdata;
- const MCRegisterInfo *MRI = DAsm->getContext().getRegisterInfo();
- int DataIdx = AMDGPU::getNamedOperandIdx(Opc, DataName);
- if ((int)Inst.getNumOperands() == DataIdx) {
- int DstIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::vdst);
- if (IsAGPROperand(Inst, DstIdx, MRI))
- Imm |= 512;
- }
-
- if (TSFlags & SIInstrFlags::DS) {
- int Data2Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::data1);
- if ((int)Inst.getNumOperands() == Data2Idx &&
- IsAGPROperand(Inst, DataIdx, MRI))
- Imm |= 512;
- }
- }
return addOperand(Inst, DAsm->decodeSrcOp(Opw, Imm | 256));
}
|
rampitec
approved these changes
Sep 10, 2025
Collaborator
rampitec
left a comment
rampitec
left a comment
There was a problem hiding this comment.
LGTM given that nothing has failed.
arsenm
deleted the
users/arsenm/amdgpu/remove-most-av-ldst-decoder-code
branch
Collaborator
|
I literally think we need to start using tcov to find dead code. It is just too much legacy to spot it with naked eyes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was additional hacking around using incorrect register class
constraints for paired data operands. I'm not really sure why we
need any of what's left. In particular the IS_VGPR special case
seems backwards from how the encoding works.